-
-
Notifications
You must be signed in to change notification settings - Fork 189
ENH: create a dataset of pre-registered motors. #814
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
… currently with the motors/ directory, which was copied from the data/motors folder
…om_dataset() to use rocketpy.datasets.motors
Well, I believe it would be nice if the installation was optional. I think it is worth to search how to do that. One my side, there are two main concerns: |
@Gui-FernandesBR, ok, I'll search for a nice way to make it optional and discuss it in the weekly. I know that it will involve another member of the team that has access to the PyPI configuration. Regarding your concerns: 1 - There are ways, but the team and I discussed in the weekly that creating the datasets directory would be a good choice because:
2 - I thought it would fit better in the utilities because there may be future functions that also use the |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #814 +/- ##
===========================================
+ Coverage 79.11% 80.13% +1.02%
===========================================
Files 96 97 +1
Lines 11575 12049 +474
===========================================
+ Hits 9158 9656 +498
+ Misses 2417 2393 -24 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a dataset of pre-registered motors into RocketPy by adding 63 new .eng files along with utility functions to list, load, and display the available motors. Additionally, the documentation has been updated to explain the new functions and dataset usage, and the CHANGELOG has been amended accordingly.
Reviewed Changes
Copilot reviewed 69 out of 69 changed files in this pull request and generated no comments.
File | Description |
---|---|
rocketpy/datasets/motors/* | New .eng files added for multiple motor models |
docs/user/motors/genericmotor.rst | Documentation updated to include new functions and dataset info |
CHANGELOG.md | Changelog updated with the new dataset feature |
@leogrosa please try to make the dataset optional on the installation. We don't have anything of the sort in the repo right now so you will need to research a bit on how to do it |
So, I did some research and found that it's not possible to make part of this repository optional when installing via pip. The only way to achieve that would be by hosting the files elsewhere. This could be done in many ways, but I believe the most scalable and maintainable option would be to create a separate repository under the RocketPy-Team organization. We could then include it as an optional dependency in RocketPy, using pip’s We could create a repository named So, my proposal is to create a dedicated repository for each dataset. For example:
This granularity would allow us to keep each dataset truly optional, and help prevent unnecessary package size growth in the long term. The only downside is having to maintain multiple small repositories. However, since these are intended to store only static data files, setup should be straightforward. I can even prepare a private “dataset repository template” to simplify this process for future datasets. What do you guys think? |
Pull request type
Checklist
black rocketpy/ tests/
) has passed locallypytest tests -m slow --runslow
) have passed locallyCHANGELOG.md
has been updated (if relevant)Current behavior
The user had to add its own .eng file to use the method
GenericMotor.load_from_eng_file()
to create a GenericMotor from it, and RocketPy didn't have a dataset of pre-registered motors.New behavior
RocketPy now has its own dataset of pre-registered motors, containing 63 .eng files, located in
rocketpy/datasets/motors/
.Three functions are now present in
utilities.py
to help:list_motors_dataset()
: returns a list of all available motors names;load_motor_from_dataset()
: load a GenericMotor from a motor name from the dataset;show_motors_dataset()
: prints the list of available motors, useful for quick inspection in terminals and Jupyter notebooks.Breaking change
Additional information
The user guide has documentation for the three new functions:



FAILED tests/integration/test_environment.py::test_rap_atmosphere - ValueError: The chosen launch time '2025-05-05-02: UTC' is not available in the provided file. Please choose a time within the range of the file, which starts at '2025-05-06-01 UTC'.
To be discussed
pip
? For now, the size of the dataset isn't big, so the easiest path is to just add it in the repo. However, in the future, it may become big enough to make it optional. Preparing the repo right now could be a good idea -- but it would demand changes in the PyPI pipeline.